Fix Hardware Id hiding when no version detected #2266

asizon · 2020-11-04T08:34:51Z

After Betaflight logo Vue implementation, HardwareId shows also when fc is flashing. Changed to show only when firmware version is deteced.
Thanks @McGiverGim for point in this lines.

sonarqubecloud · 2020-11-04T08:36:50Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
0.0% Duplication

mikeller · 2020-11-04T14:13:37Z

src/components/betaflight-logo/BetaflightLogo.vue

        </span>
        <br />
-        <span v-if="hardwareId">
+        <span v-if="firmwareVersion && hardwareId">


This is a bit of a workaround that relies on certain conditions to be true on something unrelated firmwareVersion. As such it is likely to be broken without being detected in the future. It is probably a more maintainable approach to reset hardwareId to be empty at the same place that firmwareVersion is reset on disconnect.

I don't think this will be as easy...
I've looked at the code, and it seems this two FC/MSP variables are being filled by two different MSP commands. The problem is that we have a listener that each time we connect, including when we connect to go to DFU to flash a firmware, we read the hardwareId. The other one is only read when we really "connect" in the Configurator and then we read all the FC info.
With all of this:

Do we want to add this info to the logo when flashing or only when connected? If we want when flashing too we need to add a listener to onDisconnect or something similar if exists to erase the information.

If we want only when connected, another option, more descriptive, is to pass a variable to the Vue object indicating if we want to add FC info to the logo, that can be the associated to the variable that contains the info about if we are connected or not.

EDIT: this variable can be GUI.connected_to that contains false if not connected or the port in text form if connected. I don't know if there is other more suitable.

Yes @mikeller I'm not satisfied enough with this fix that is based on firmwareVersion, I'll give it one more review to find a better solution

Sorry, I found the time to test this with an actual board only now - since the hardwareId is based on actual information that is returned from the board, I am not sure that it is wrong to show it while flashing the firmware. After that it becomes stale, as there is no way for us to ensure that the same board is connected, especially with the flashing process involving changes between the VCP in serial mode and DFU.
So maybe, instead of hiding this when flashing, we should just make sure that hardwareId is reset at the end of flashing?
Maybe this could be improved optically by moving the Target information above the firmware version information in the top bar.

@mikeller definitelly it happens when dfu autoenable is used in firmware flasher tab, fc reboots in dfu mode and the hardwareId is recognized. If we enable dfu in setup tab al works as expected.

@asizon: Yes, this is to be expected. If the flight controller is not in DFU mode already when the flashing is started, we try to 'half-connect' in order to issue the MSP command to reboot the flight controller into DFU mode. This is when the hardware id is read. So keeping this hardware id while the subsequent flashing process is running seems to me to make sense.

@asizon: Do you want to change this so that the hardware id is shown during flashing if it is known?

Yes @mikeller im going to look at this, sorry

No reason to be sorry - this is a hobby, and everything is voluntary.

asizon · 2020-12-07T07:32:16Z

@mikeller @McGiverGim I am somewhat confused with this and I don't know how to act. Regarding letting it appear in the flashing I think it is incorrect, the hardware Id not be coherent with the flashing target, when the chip erase is finished it should disappear, perhaps directly not appear if it is disconnected.

haslinghuis · 2020-12-08T05:26:12Z

Putting FC.resetState(); on line 166 will remove hardwareId while flashing. Reason to do it this way is that hardwareId does not represent the firmware to flash but the firmware read using the MSP_BOARD_INFO command to determine the boot method (ROM/Flash) to boot the board in DFU mode in the next code section.

betaflight-configurator/src/js/protocols/stm32.js

Lines 152 to 166 in a8b43e1

    
           console.log('Looking for capabilities via MSP'); 
        
           MSP.send_message(MSPCodes.MSP_BOARD_INFO, false, false, function () { 
        
               var rebootMode = 0; // FIRMWARE 
        
               if (bit_check(FC.CONFIG.targetCapabilities, FC.TARGET_CAPABILITIES_FLAGS.HAS_FLASH_BOOTLOADER)) { 
        
                   // Board has flash bootloader 
        
                   GUI.log(i18n.getMessage('deviceRebooting_flashBootloader')); 
        
                   console.log('flash bootloader detected'); 
        
                   rebootMode = 4; // MSP_REBOOT_BOOTLOADER_FLASH 
        
               } else { 
        
                   GUI.log(i18n.getMessage('deviceRebooting_romBootloader')); 
        
                   console.log('no flash bootloader detected'); 
        
                   rebootMode = 1; // MSP_REBOOT_BOOTLOADER_ROM; 
        
               }

asizon · 2020-12-08T08:40:07Z

Thanks for point out this @haslinghuis , moved FC.resetState to stm32.js file.

mikeller · 2020-12-08T09:50:10Z

@asizon:

@mikeller @McGiverGim I am somewhat confused with this and I don't know how to act. Regarding letting it appear in the flashing I think it is incorrect, the hardware Id not be coherent with the flashing target, when the chip erase is finished it should disappear, perhaps directly not appear if it is disconnected.

Not sure what you mean by this - the hardware id is the id that is stored on the MCU, and is meant to be used as an indicator for users to help them select the correct target to flash. Right now we are not enforcing coherence with the firmware that is flashed, but at some point we might want to use it to warn the user if they try to flash an incorrect target, or even read it when entering the firmware flasher tab and use it to pre-select the correct target.

Not sure why you think that not showing correct information is an improvement - to me this is going into the wrong direction.

There is one exception: The only way to remove the hardware id is to flash with 'full chip erase' checked, so in this case we might want to suppress the hardware id that is being erased.

asizon · 2020-12-08T09:55:48Z

But @mikeller ,we have a problem with your suggested aproach, for the moment we can get hardwareId only if the user uses the auto-dfu change pressing flash button, because it not resetState. But if the user go to dfu using setup tab or cli, it resetState when disconect and hardwareId never appears.

mikeller · 2020-12-08T10:16:08Z

@asizon: Yes, the display at the moment is only opportunistic. But when it is displayed it is correct, so there is no reason to suppress it. Probably the simplest way to deal with this is to issue FC.reset() when a USB disconnect is detected, so that the information is not persisted after the device has been disconnected.

asizon · 2020-12-08T10:21:16Z

Interesting @mikeller , lets see what can I do. I also think that being able to see the hardwareId (thanks to the reactivity vue, I think), is an improvement.

mikeller · 2020-12-08T10:30:51Z

My thinking too @asizon.
This feature has been in the firmware for some time, but until now it is not used for much.
I think it will be good improvement in user friendliness if we can add a dedicated way to query it in the firmware flasher tab (either on connect, or with a 'Detect Hardware' button), and then use the information to pre-select the correct target.

sonarqubecloud · 2020-12-08T11:01:53Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

asizon · 2020-12-08T11:03:14Z

wow this feature sound good @mikeller , now hardwareId is removed when flash finished and check for new devices, lets see if we can some exception when erase chip is enabled, any ideas?

mikeller · 2020-12-08T12:04:17Z

Looking at this, this seems to be correct the way it is - the hardware target id flashes only quickly when 'Flash' is clicked, as the flight controller reboots into DFU mode immediately, and this is recognised as a disconnect / reconnect. So I think this is correct (but not super helpful) for now, and can be merged.

If you are interested to go deeper with this in a next pull request, you could try using the manufacturer / target id stored on the MCU that you get when rebooting into DFU at the beginning of the flashing process, and compare it against the selected target. If they don't match we can then show a dialog with a warning telling the user that the target they are about to flash is different from the target their hardware identifies as, and let them choose if they want to continue or abort.

Hide hardwareId when no version detected

b518509

McGiverGim previously approved these changes Nov 4, 2020

View reviewed changes

mikeller reviewed Nov 4, 2020

View reviewed changes

Force FC status reset when flashing

5b836e0

asizon dismissed McGiverGim’s stale review via 5b836e0 December 7, 2020 08:20

Move resetState to stm32.js

95665de

asizon force-pushed the hide_hardwareid branch from 83ba9bc to 95665de Compare December 8, 2020 08:37

Added condition to avoid duplication when is connected

67be783

Remove hardwareId when checks for usb devices

883bb9c

asizon force-pushed the hide_hardwareid branch from 23b86bd to 883bb9c Compare December 8, 2020 10:59

mikeller added this to the 10.8.0 milestone Dec 8, 2020

mikeller added the RN: BUGFIX label Dec 8, 2020

mikeller merged commit 317f937 into betaflight:master Dec 8, 2020

mikeller modified the milestones: 10.8.0, 10.7.1 Oct 10, 2021

mikeller added a commit that referenced this pull request Oct 10, 2021

Merge pull request #2266 from Asizon/hide_hardwareid

684647f

asizon deleted the hide_hardwareid branch February 15, 2022 09:21

Uh oh!

Fix Hardware Id hiding when no version detected #2266

Fix Hardware Id hiding when no version detected #2266

Uh oh!

Conversation

asizon commented Nov 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Nov 4, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

McGiverGim Nov 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asizon commented Dec 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haslinghuis commented Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asizon commented Dec 8, 2020

Uh oh!

mikeller commented Dec 8, 2020

Uh oh!

asizon commented Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikeller commented Dec 8, 2020

Uh oh!

asizon commented Dec 8, 2020

Uh oh!

mikeller commented Dec 8, 2020

Uh oh!

sonarqubecloud bot commented Dec 8, 2020

Uh oh!

asizon commented Dec 8, 2020

Uh oh!

mikeller commented Dec 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asizon commented Nov 4, 2020 •

edited

Loading

McGiverGim Nov 4, 2020 •

edited

Loading

asizon commented Dec 7, 2020 •

edited

Loading

haslinghuis commented Dec 8, 2020 •

edited

Loading

asizon commented Dec 8, 2020 •

edited

Loading